Skip to content

Fixes #2073. Can no longer nest Application.MainLoop.Invoke.#2075

Closed
BDisp wants to merge 3 commits intogui-cs:developfrom
BDisp:menu-action-invoke--fix
Closed

Fixes #2073. Can no longer nest Application.MainLoop.Invoke.#2075
BDisp wants to merge 3 commits intogui-cs:developfrom
BDisp:menu-action-invoke--fix

Conversation

@BDisp
Copy link
Copy Markdown
Collaborator

@BDisp BDisp commented Oct 3, 2022

Fixes #2073 - View action must be called without using the Application.MainLoop.Invoke to avoid been blocked by the idle handlers..

Pull Request checklist:

  • I've named my PR in the form of "Fixes #issue. Terse description."
  • My code follows the style guidelines of Terminal.Gui - if you use Visual Studio, hit CTRL-K-D to automatically reformat your files before committing.
  • My code follows the Terminal.Gui library design guidelines
  • I ran dotnet test before commit
  • I have made corresponding changes to the API documentation (using /// style comments)
  • My changes generate no new warnings
  • I have checked my code and corrected any poor grammar or misspellings
  • I conducted basic QA to assure all features are working

tig
tig reviewed Oct 5, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@tig tig left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pattern seems to exist elsewhere in the project. E.g.

image

Should this PR fix those too?

Do we know that this pattern doesn't exist elsewhere (i literally just searched for AddIdle).

What if 3rd party apps use AddIdle like this?

@BDisp
Copy link
Copy Markdown
Collaborator Author

BDisp commented Oct 6, 2022

I really don't know why the unit tests aren't successfully passed. I tested with VSCode on Windows and on the .devcontainer and all the test passed. I'll revert all and only submit the actions without the AddIdle. @montekarlos if you know why this is happening please help, thanks.

@BDisp BDisp force-pushed the menu-action-invoke--fix branch from ff86eed to e253958 Compare October 12, 2022 23:10
@BDisp BDisp force-pushed the menu-action-invoke--fix branch from e253958 to ddab1eb Compare October 13, 2022 00:00
@BDisp
Copy link
Copy Markdown
Collaborator Author

BDisp commented Oct 13, 2022

Well, in addition to the clipboard issue the actions must be invoke through the Application.MainLoop.Invoke which will call the Application.MainLoop.AddIdle, but will be dispatched without waiting by the return value.

@tig
Copy link
Copy Markdown
Member

tig commented Oct 14, 2022

Given @tznind's #2083, this PR can be closed, right?

@BDisp
Copy link
Copy Markdown
Collaborator Author

BDisp commented Oct 14, 2022

As I said I can't check that before Tuesday.

@tig
Copy link
Copy Markdown
Member

tig commented Oct 14, 2022

As I said I can't check that before Tuesday.

No problem. I'm just doing some housecleaning.

@BDisp BDisp mentioned this pull request Oct 19, 2022
Closed
@BDisp BDisp closed this Oct 19, 2022
@BDisp
Copy link
Copy Markdown
Collaborator Author

BDisp commented Oct 19, 2022

Given @tznind's #2083, this PR can be closed, right?

Right, closing.

@BDisp BDisp mentioned this pull request Oct 20, 2022
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tig tig tig left review comments

@migueldeicaza migueldeicaza Awaiting requested review from migueldeicaza

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Can no longer nest Application.MainLoop.Invoke (since 1.8.2)

2 participants

@BDisp @tig